Stop overwriting all inbox items with onboarding message#2740
Conversation
|
Warning Rate limit exceeded@Crabcyborg has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 7 minutes and 35 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @classes/controllers/FrmOnboardingWizardController.php:
- Line 471: Ensure $inbox_messages is an array before assigning to
$inbox_messages['onboarding_wizard']; validate the parameter returned by the
option/filter (e.g., option_frm_inbox) and coerce or initialize it to an array
if it's null/false/non-array (for example by checking is_array($inbox_messages)
and setting $inbox_messages = array() when not); update the code around the
$inbox_messages['onboarding_wizard'] assignment in FrmOnboardingWizardController
to perform this guard so no PHP warning/fatal occurs when non-array values are
returned.
🧹 Nitpick comments (1)
classes/controllers/FrmOnboardingWizardController.php (1)
469-469: Consider inlining the$messagevariable.The
$messagevariable is only used twice in close proximity. You could inline it directly in the array definition to reduce variable declarations.♻️ Proposed refactor
public static function add_wizard_to_floating_links( $inbox_messages ) { - $message = __( 'Welcome to Formidable Forms! Click here to run the Onboarding Wizard and it will guide you through the basic settings and get you started in 2 minutes.', 'formidable' ); + if ( ! is_array( $inbox_messages ) ) { + $inbox_messages = array(); + } + $message = __( 'Welcome to Formidable Forms! Click here to run the Onboarding Wizard and it will guide you through the basic settings and get you started in 2 minutes.', 'formidable' ); + $inbox_messages['onboarding_wizard'] = array( 'subject' => esc_html__( 'Begin With Ease!', 'formidable' ), 'message' => esc_html( $message ),Note: Keep the variable as-is if you prefer the clarity, especially since
$messageis used in both 'message' and 'slidein' keys.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
classes/controllers/FrmOnboardingWizardController.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Cypress
- GitHub Check: PHP 8 tests in WP 6.9
- GitHub Check: PHP 7.4 tests in WP 6.9
- GitHub Check: PHP 7.4 tests in WP 6.9
- GitHub Check: PHP 8 tests in WP 6.9
- GitHub Check: Cypress
🔇 Additional comments (1)
classes/controllers/FrmOnboardingWizardController.php (1)
468-481: Fix correctly preserves existing inbox messages.The refactored implementation now properly adds the onboarding wizard message to the existing
$inbox_messagesarray instead of overwriting all messages. This resolves the bug described in the PR objectives.
shervElmi
left a comment
shervElmi
left a comment
There was a problem hiding this comment.
It looks good to me! 🚀
Thanks @Crabcyborg.
|
Thanks Sherv! |
…tems_with_onboarding_message Stop overwriting all inbox items with onboarding message
2 participants
I'm doing some code analysis and it found that
$inbox_messagesis never used.But it should be if this is used on the
option_frm_inboxfilter, as it overwrites all other messages the way it was written.